feat(ContentSwitcher): add icon-only ContentSwitcher #13378

tw15egan · 2023-03-20T18:58:31Z

Adds in a new icon-only ContentSwitcher

Changelog

New

IconSwitch component was created, and rendered as a child of ContentSwitcher. When this is used instead of Switch, it will render an IconButton.

<ContentSwitcher>
    <IconSwitch name="one" text="Table of Contents">
      <TableOfContents />
    </IconSwitch>
    <IconSwitch name="two" text="Workspace Test">
      <Workspace />
    </IconSwitch>
    <IconSwitch name="three" text="View Mode">
      <ViewMode_2 />
    </IconSwitch>
  </ContentSwitcher>

The name passed into IconSwitch will render the Tooltip text. This will also set the aria-label on the IconButton
Tests and snapshots added for the new IconSwitch
Styles for the new icon-only ContentSwitcher

Changed

ContentSwitcher now passes along size to its children.
Updated the size prop types to valid v11 sizes.
Adjusted styles for the new icon-only ContentSwitcher
Added a wrapperClasses prop to IconButton. I needed a way to surface selected + disabled states to the outermost wrapper, and adding one in IconSwitch was causing the role="tab" be too far removed from its parent, and causing screenreader issues (Each button read Tab 1 of 1). Since it is now just 1 level removed, the screen reader announces the number of tabs correctly.

Removed

Removed the with layer snapshot and replaced it with the new icon-only story. The with layer story has no visual differences from the default story.

Testing / Reviewing

Go to ContentSwitcher and check out the icon-only story.

Ensure it matches the spec, and tooltips appear.
Ensure sizes are able to change, and supports both selection modes.
Try disabling an icon button and ensure it works.

Make sure there are no regressions to the standard ContentSwitcher

netlify · 2023-03-20T19:03:05Z

✅ Deploy Preview for carbon-components-react ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`9677d47`
🔍 Latest deploy log	https://app.netlify.com/sites/carbon-components-react/deploys/643423da437d9e0008da920a
😎 Deploy Preview	https://deploy-preview-13378--carbon-components-react.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

netlify · 2023-03-20T19:03:19Z

✅ Deploy Preview for carbon-elements ready!

Name	Link
🔨 Latest commit	`9677d47`
🔍 Latest deploy log	https://app.netlify.com/sites/carbon-elements/deploys/643423da38bd960008f2b891
😎 Deploy Preview	https://deploy-preview-13378--carbon-elements.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

packages/react/src/components/ContentSwitcher/ContentSwitcher.js

tay1orjones

Looks great! 🎉 🏆

tay1orjones · 2023-03-24T19:11:14Z

packages/react/src/components/ContentSwitcher/ContentSwitcher.js

@@ -141,9 +140,14 @@ export default class ContentSwitcher extends React.Component {
      ...other
    } = this.props;

+    const isIconOnly = React.Children.map(children, (child) => {
+      return child.type.displayName === 'IconSwitch';


Just want to make a note here that this locks the API so consumers can only use IconSwitch. It could prove problematic if someone were to to roll their own IconSwitch component and names it something different. Let's cross that bridge if we come to it though - no need to over optimize for it right now imo

laurenmrice

Looking really good!! Just a couple of comments.

I noticed there is a slight shadow or gray color bug on the left side of the first section when its selected

When the last section is selected and has focus, the focus border does not completely overlap the selected color layer on the right side.

The icon color in the sections that are unselected should be $icon-primary.
The Large size content switcher should have 20px icons instead of 16px.

aledavila

LGTM pending lauren's style issue

packages/styles/scss/components/content-switcher/_content-switcher.scss

laurenmrice

Everything looks good except for the focus state in Safari only. The inset padding becomes inconsistent in some content switcher sections in the different sizes.

laurenmrice

Looks great to me! Thanks TJ! ✅

tw15egan force-pushed the icon-only-content-switcher branch 2 times, most recently from 1a69cd7 to 2d326fe Compare March 22, 2023 16:14

tw15egan commented Mar 22, 2023

View reviewed changes

packages/react/src/components/ContentSwitcher/ContentSwitcher.js Outdated Show resolved Hide resolved

tw15egan force-pushed the icon-only-content-switcher branch 3 times, most recently from c458e3f to 9ac8e18 Compare March 23, 2023 16:32

tw15egan marked this pull request as ready for review March 23, 2023 16:53

tw15egan requested review from a team as code owners March 23, 2023 16:53

tw15egan requested review from tay1orjones, aledavila and laurenmrice March 23, 2023 16:53

tw15egan added status: ready for review 👀 status: visual review 🔍 labels Mar 23, 2023

tw15egan force-pushed the icon-only-content-switcher branch from aeca559 to 95a9c42 Compare March 23, 2023 18:00

tay1orjones approved these changes Mar 24, 2023

View reviewed changes

tay1orjones added the status: one more review 👀 label Mar 24, 2023

laurenmrice requested changes Mar 28, 2023

View reviewed changes

aledavila approved these changes Mar 29, 2023

View reviewed changes

packages/styles/scss/components/content-switcher/_content-switcher.scss Show resolved Hide resolved

tw15egan force-pushed the icon-only-content-switcher branch 3 times, most recently from d57b8b9 to 8cd5b3d Compare March 31, 2023 11:38

tw15egan requested a review from laurenmrice March 31, 2023 12:08

laurenmrice requested changes Apr 4, 2023

View reviewed changes

tw15egan added 4 commits April 10, 2023 12:19

feat(ContentSwitcher): add icon-only ContentSwitcher

31133ec

feat(ContentSwitcher): add icon-only ContentSwitcher functionality

2067a8d

style(ContentSwitcher): add styles for icon-only ContentSwitcher

c1191b3

test(ContentSwitcher): add e2e tests for icon-only ContentSwitcher

1a4b4d4

tw15egan added 14 commits April 10, 2023 12:20

fix(Switch): remove extra aria-label

57b516b

feat(IconSwitch): refactor new features to IconSwitch component

02aa257

fix(ContentSwitcher): remove isIconOnly prop

5d44cd3

chore(test): update snapshots

94f7e5f

feat(IconSwitch): add IconButton props, refactor

3c33ab8

test(ContentSwitcher): update snapshots

2f91e68

refactor(IconSwitch): remove defaultProps

e0d32ec

test(IconSwitch): add tests

526f087

refactor(IconSwitch): remove wrapper, add wrapperClasses to IconButton

2ad20ff

chore(test): move test back

ce3c38b

chore(test): move test back

840025a

style(ContentSwitcher): make fixes based on design review

7694702

style(ContentSwitcher): fix focus issue

759a8d1

test(e2e): add withLayer story, e2e tests

dbf5359

tw15egan force-pushed the icon-only-content-switcher branch from 2df73f3 to dbf5359 Compare April 10, 2023 10:26

tw15egan requested a review from laurenmrice April 10, 2023 10:28

Merge branch 'main' into icon-only-content-switcher

d0d39ea

laurenmrice approved these changes Apr 10, 2023

View reviewed changes

tw15egan added status: ready to merge 🎉 and removed status: ready for review 👀 status: one more review 👀 status: visual review 🔍 labels Apr 10, 2023

tw15egan and others added 2 commits April 10, 2023 16:50

Merge branch 'main' into icon-only-content-switcher

69acca9

Merge branch 'main' into icon-only-content-switcher

9677d47

kodiakhq bot merged commit e49be55 into carbon-design-system:main Apr 10, 2023

francinelucca mentioned this pull request Apr 12, 2023

Icon content switcher: add implementation #12461

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ContentSwitcher): add icon-only ContentSwitcher #13378

feat(ContentSwitcher): add icon-only ContentSwitcher #13378

tw15egan commented Mar 20, 2023 •

edited

Loading

netlify bot commented Mar 20, 2023 •

edited

Loading

netlify bot commented Mar 20, 2023 •

edited

Loading

tay1orjones left a comment

tay1orjones Mar 24, 2023

laurenmrice left a comment •

edited

Loading

aledavila left a comment •

edited

Loading

laurenmrice left a comment

laurenmrice left a comment

feat(ContentSwitcher): add icon-only ContentSwitcher #13378

feat(ContentSwitcher): add icon-only ContentSwitcher #13378

Conversation

tw15egan commented Mar 20, 2023 • edited Loading

Changelog

Testing / Reviewing

netlify bot commented Mar 20, 2023 • edited Loading

✅ Deploy Preview for carbon-components-react ready!

netlify bot commented Mar 20, 2023 • edited Loading

✅ Deploy Preview for carbon-elements ready!

tay1orjones left a comment

Choose a reason for hiding this comment

tay1orjones Mar 24, 2023

Choose a reason for hiding this comment

laurenmrice left a comment • edited Loading

Choose a reason for hiding this comment

aledavila left a comment • edited Loading

Choose a reason for hiding this comment

laurenmrice left a comment

Choose a reason for hiding this comment

laurenmrice left a comment

Choose a reason for hiding this comment

tw15egan commented Mar 20, 2023 •

edited

Loading

netlify bot commented Mar 20, 2023 •

edited

Loading

netlify bot commented Mar 20, 2023 •

edited

Loading

laurenmrice left a comment •

edited

Loading

aledavila left a comment •

edited

Loading